Codecov Report

Merging #29 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
+ Coverage   99.34%   99.35%   +0.01%     
==========================================
  Files           3        3              
  Lines         153      156       +3     
==========================================
+ Hits          152      155       +3     
  Misses          1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3e199c...53adde6. Read the comment docs.

Thanks for the contribution. I'll review this PR on weekends.

@cjroth

It seems you are trying to use an instance pk instead of a relation pk.

What about cases, when we have User model and, for example, Profile model. Profile is one to one relation to User, but not mandatory.
So, we can have a user with pk=100 and a profile with pk=90.

@cjroth sorry to bother, but is still in a good state/up to date? I can give it a test if so! I'm getting some problems with the parent relationship not being created and believe this should fix it.

EDIT: Tested it against this and I got a "field is required" error creating the parent:

# models
class Competition(models.Model):
    created_by = models.ForeignKey(settings.AUTH_USER_MODEL, null=True, blank=True, on_delete=models.SET_NULL, related_name="competitions")
    created_when = models.DateTimeField(auto_now_add=True)
    title = models.CharField(max_length=256)


class Page(models.Model):
    competition = models.ForeignKey(Competition, related_name='pages', on_delete=models.DO_NOTHING)
    title = models.TextField(max_length=255)
    content = models.TextField()
    index = models.PositiveIntegerField()


# Serializers
class PageSerializer(serializers.ModelSerializer):  # also tried WriteableNestedModelSerializer here
    # *NOTE* The competition property has to be replicated at the end of the file
    # after the CompetitionSerializer class is declared
    # competition = CompetitionSerializer(many=True)

    class Meta:
        model = Page
        fields = (
            'competition',
            'title',
            'content',
            'index',
        )


class CompetitionSerializer(WritableNestedModelSerializer):
    created_by = serializers.SerializerMethodField(read_only=True)
    pages = PageSerializer(many=True)

    class Meta:
        model = Competition
        fields = (
            'id',
            'title',
            'created_by',
            'pages',
        )

PageSerializer.competition = CompetitionSerializer(many=True)  # also tried with explicit source definition

Given:

{
    "title": "asdf",
    "pages": [{"title": "test", "content": "test","index":"1"}]
}

Result is:

{
    "pages": [
        {
            "competition": [
                "This field is required."
            ]
        }
    ]
}

I could be doing something else stupid and/or misunderstanding this, as well...

I haven't looked at it since this PR but we are using my branch with no issues at the moment. I will try to look at it when I get some time. Sorry to be MIA!

So to anyone else who runs into my problem, I think I just misunderstood how drf-writable-nested works. I needed to add the required=False kwargs and let the module do its job! Validation was stopping it, drf-writable-nested doesn't remove this validation for you.

class PageSerializer(WritableNestedModelSerializer):
    class Meta:
        model = Page
        fields = (
            'competition',
        )
        extra_kwargs = {
            # This was key!
            "competition": {"required": False}
        }

Thanks!

I had a similar situation, where removing the field from class Meta made the difference.

The extra_kwargs didn't seem to matter. Rather, removing competition from the fields was the only way we could get it to work.

There's some perhaps inconsistent or unexpected behavior that we should write out in the docs. I'll try to do that shortly.

On another note, is #29 ready to be merged?

I think this MR contains something that can brake another part of logic. See my comments to this MR.